Skip to content

fix: wrong order of resolving function expression and operands expression for call_indirect#2994

Open
Changqing-JING wants to merge 2 commits intoAssemblyScript:mainfrom
Changqing-JING:bugfix/2989
Open

fix: wrong order of resolving function expression and operands expression for call_indirect#2994
Changqing-JING wants to merge 2 commits intoAssemblyScript:mainfrom
Changqing-JING:bugfix/2989

Conversation

@Changqing-JING
Copy link
Copy Markdown
Contributor

@Changqing-JING Changqing-JING commented Mar 21, 2026

Fixes #2989

Changes proposed in this pull request:
Fix wrong order of resolving function expression and operands expression for call_indirect

  • I've read the contributing guidelines
  • I've added my name and email to the NOTICE file

@Changqing-JING Changqing-JING changed the title Fix wrong order of resolving function expression and operands expression for call_indirect fix: wrong order of resolving function expression and operands expression for call_indirect Mar 21, 2026
Changqing-JING added a commit to wasm-ecosystem/warpo that referenced this pull request Mar 23, 2026
@Changqing-JING
Copy link
Copy Markdown
Contributor Author

Can some maintainer help to review this PR?

@MaxGraey MaxGraey requested review from CountBleck and HerrCai0907 and removed request for HerrCai0907 March 25, 2026 13:32
@HerrCai0907
Copy link
Copy Markdown
Member

It makes the semantics of as and ts more consistent, with a slight performance loss, yet I believe this trade-off is worthwhile. @MaxGraey WDYT?

@MaxGraey
Copy link
Copy Markdown
Member

I'm not sure. I think AssemblyScript provides much more obvious and logical approach. JavaScript has historically had a lot of quirks, and I think this is one of them. Also, it's breaking change

@Changqing-JING Changqing-JING deleted the bugfix/2989 branch April 26, 2026 02:03
@Changqing-JING Changqing-JING restored the bugfix/2989 branch April 26, 2026 02:05
@Changqing-JING
Copy link
Copy Markdown
Contributor Author

Changqing-JING commented Apr 26, 2026

@MaxGraey In my opinion, the assemblyscript need to follow typescript/javascript design

  1. It's a javascript spec compliance. ECMAScript §13.3.6.1 mandates callee evaluation before arguments. AS claims to be a TS subset — deviating here silently produces wrong results.

  2. It's not a quirk, evaluating the callee before arguments is the common convention across major languages, aligning with most programmers' mental model.

    Python — guaranteed left-to-right evaluation:

    def log_a(x):
        global value
        value = 100
    
    def log_b(x):
        global value
        value = 200
    
    value = 0
    fn = log_a
    
    def foo(x):
        global fn
        fn = log_b
        return x
    
    fn(foo(42))
    print(f"value = {value}")  # → 100

    Java — same behavior:

     import java.util.function.IntConsumer;
    
    public class EvalOrderTest {
        static int value = 0;
        static IntConsumer fn;
    
        static void logA(int x) { value = 100; }
        static void logB(int x) { value = 200; }
    
        static int foo(int x) {
            fn = EvalOrderTest::logB;
            return x;
        }
    
        public static void main(String[] args) {
            fn = EvalOrderTest::logA;
            fn.accept(foo(42));
            System.out.println("value = " + value);  // → 100
        }
    }

    Both Python and Java guarantee value = 100 — AS is the outlier among languages that claim defined semantics.

  3. Negligible perf cost. The allTrivial fast path skips the extra temp local when no side effects exist, which is the common case.

  4. Not a real breaking change. Code relying on the wrong order is already buggy. Fixing correctness doesn't break correct programs.

@MaxGraey
Copy link
Copy Markdown
Member

Oh, now I see. Сallee should be saved before args run. Otherwise, side effects can swap the function under your feet, which is pretty footgun-y.

@MaxGraey
Copy link
Copy Markdown
Member

MaxGraey commented Apr 26, 2026

It will be great to add smoke test for compiler which check correct order in output.

Something like:

function origCallback(x: i32): void {
  trace(`called origCallback with ${x}`);
}

function replacedCallback(x: i32): void {
  trace(`called replacedCallback with ${x}`);
}

let cb = origCallback;

function newCallback(): i32 {
  trace("inside newCallback, replacing cb");
  cb = replacedCallback;
  return 42;
}

trace("before call");
cb(newCallback());
trace("after call");

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Function reference and parameter are resolved in wrong order.

3 participants